docs: minor cleanup to simulation docs #21777
Conversation
WalkthroughWalkthroughThe changes in this pull request enhance the documentation and functionality related to implementing store decoders and simulation functions within the Cosmos SDK. Key updates include new references for modules, streamlined instructions for overriding package parameters, clarifications on governance proposals, and modifications to the store decoder registration method. Additionally, a new command for fuzz testing in simulations has been introduced, and various references have been updated to ensure alignment with the latest codebase. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Recent review detailsConfiguration used: .coderabbit.yml Files selected for processing (1)
Additional context usedPath-based instructions (1)
LanguageTool
Additional comments not posted (5)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
types/simulation/collections.go (1)
Line range hint
49-51: Consider returning an error instead of panicking for unexpected keys.Instead of using
panicwhen an unexpected key is encountered, consider returning an error. This would improve error handling and prevent potential crashes in case of unexpected keys.- panic(fmt.Errorf("unexpected key %X (%s)", kvA.Key, kvA.Key)) + return "", fmt.Errorf("unexpected key %X (%s)", kvA.Key, kvA.Key)docs/learn/advanced/12-simulation.md (2)
16-16: Consider replacing the colloquial expression.The expression "come in handy" is rather colloquial. Consider replacing it with a more formal alternative to improve the tone of the documentation.
For example, you could rephrase the sentence as follows:
-This comes in handy when trying to reproduce bugs that were generated in the provided operations (randomized or not). +This is particularly useful when attempting to reproduce bugs that were generated in the provided operations (randomized or not).Tools
LanguageTool
[style] ~16-~16: The expression “come in handy” is rather colloquial. Consider replacing it to add a more formal tone to your writing.
Context: ... the chain that's being simulated. This comes in handy when trying to reproduce bugs that were...(COME_IN_HANDY)
23-29: LGTM, but consider replacing the uncommon contraction.The addition of the
FuzzFullAppSimulationcommand enhances the documentation by highlighting an additional functionality for improved testing.However, the contraction "there're" is uncommon in written English. Consider replacing it with a more formal alternative to improve the tone of the documentation.
For example, you could rephrase the sentence as follows:
-Tests that there're no `panics` on the simulation. +Tests that there are no `panics` during the simulation.Note: The loose punctuation marks flagged by the static analysis tool are intentional as they are part of a list format in markdown.
Tools
LanguageTool
[uncategorized] ~23-~23: Loose punctuation mark.
Context: ...erent failure type: *AppImportExport: The simulator exports the initial app s...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~23-~23: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... simulator exports the initial app state and then it creates a new app with the expo...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~24-~24: Loose punctuation mark.
Context: ...the stores. *AppSimulationAfterImport: Queues two simulations together. The fi...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~25-~25: Loose punctuation mark.
Context: ...om a live chain. *AppStateDeterminism: Checks that all the nodes return the sa...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~26-~26: Loose punctuation mark.
Context: ... the same order. *BenchmarkInvariants: Analysis of the performance of running ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~27-~27: Loose punctuation mark.
Context: ...al supply tracker. *FullAppSimulation: General simulation mode. Runs the chain...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~27-~27: The contraction ‘there’re’ is uncommon in written English.
Context: ...or a given number of blocks. Tests that there're nopanicson the simulation. It does ...(THERE_RE_CONTRACTION_UNCOMMON)
[uncategorized] ~28-~28: Loose punctuation mark.
Context: ...t benchmarked. *FuzzFullAppSimulation: Runs general simulation mode with the [...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~29-~29: Loose punctuation mark.
Context: ... to find panics. *AppStateDeterminism: Runs a few seeds many times to test tha...(UNLIKELY_OPENING_PUNCTUATION)
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (4)
- docs/build/building-modules/14-simulator.md (3 hunks)
- docs/learn/advanced/12-simulation.md (2 hunks)
- simsx/README.md (2 hunks)
- types/simulation/collections.go (1 hunks)
Files skipped from review due to trivial changes (1)
- simsx/README.md
Additional context used
Path-based instructions (3)
types/simulation/collections.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.docs/learn/advanced/12-simulation.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/build/building-modules/14-simulator.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
LanguageTool
docs/learn/advanced/12-simulation.md
[style] ~16-~16: The expression “come in handy” is rather colloquial. Consider replacing it to add a more formal tone to your writing.
Context: ... the chain that's being simulated. This comes in handy when trying to reproduce bugs that were...(COME_IN_HANDY)
[uncategorized] ~23-~23: Loose punctuation mark.
Context: ...erent failure type: *AppImportExport: The simulator exports the initial app s...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~23-~23: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... simulator exports the initial app state and then it creates a new app with the expo...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~24-~24: Loose punctuation mark.
Context: ...the stores. *AppSimulationAfterImport: Queues two simulations together. The fi...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~25-~25: Loose punctuation mark.
Context: ...om a live chain. *AppStateDeterminism: Checks that all the nodes return the sa...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~26-~26: Loose punctuation mark.
Context: ... the same order. *BenchmarkInvariants: Analysis of the performance of running ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~27-~27: Loose punctuation mark.
Context: ...al supply tracker. *FullAppSimulation: General simulation mode. Runs the chain...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~27-~27: The contraction ‘there’re’ is uncommon in written English.
Context: ...or a given number of blocks. Tests that there're nopanicson the simulation. It does ...(THERE_RE_CONTRACTION_UNCOMMON)
[uncategorized] ~28-~28: Loose punctuation mark.
Context: ...t benchmarked. *FuzzFullAppSimulation: Runs general simulation mode with the [...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~29-~29: Loose punctuation mark.
Context: ... to find panics. *AppStateDeterminism: Runs a few seeds many times to test tha...(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~41-~41: Possible agreement error.
Context: ...lated. The list of available parameters are listed [here](https://github.com/cosmos...(AGREEMENT_SENT_START)
docs/build/building-modules/14-simulator.md
[uncategorized] ~71-~71: The preposition ‘to’ seems more likely in this position.
Context: ...tions can be executed like normal tests in Go from the shell or within an IDE. Mak...(AI_HYDRA_LEO_REPLACE_IN_TO)
Additional comments not posted (9)
types/simulation/collections.go (1)
Line range hint
13-54: LGTM! The function implementation aligns with the PR objectives.The
NewStoreDecoderFuncFromCollectionsSchemafunction provides a convenient way to decode key-value pairs based on the module's collections schema. It ensures that the key-value pairs have matching prefixes and uses the appropriate value codec for decoding.The function implementation adheres to the Uber Go Style Guide, with clear variable names, proper error handling, and concise code structure.
docs/learn/advanced/12-simulation.md (2)
10-10: LGTM!Updating the link to the latest commit ensures that users access the most up-to-date documentation.
14-14: LGTM!Removing unnecessary line breaks enhances readability without altering the content's meaning.
docs/build/building-modules/14-simulator.md (6)
41-42: Excellent addition of the Bank module example for store decoders.The reference to the Bank module example provides valuable guidance for modules utilizing the
collectionspackage. This addition enhances the documentation by offering a concrete implementation for developers to refer to.
66-66: Good update to the reference for setting weights in simulations.The updated reference points to a more current and relevant code example for defining weights. This change ensures that the documentation is in sync with the latest codebase, providing accurate guidance to developers.
74-76: Appropriate update to the reference for overriding package parameters.The updated reference directs users to the current location of the relevant code for overriding package
simappparams. This change ensures that the documentation accurately reflects the latest codebase structure, improving clarity for developers.
81-85: Excellent update to the reference and clarification for registering governance proposal messages.The updated reference points users to the current location of the relevant code for registering governance proposal messages. Additionally, the clarification that modules must register messages for governance proposals emphasizes this important requirement. These changes ensure that the documentation is accurate, up-to-date, and provides clear instructions for developers.
92-92: Appropriate update to the reference for integrating simulation functions into the module pattern.The updated reference directs users to the current location of the relevant code for integrating simulation functions. This change ensures that the documentation accurately reflects the latest codebase structure, providing developers with the most up-to-date information.
138-138: Good update to the reference for deterministic behavior integration.The updated reference points users to the current location of the relevant code for integrating deterministic behavior. This change ensures that the documentation is in sync with the latest codebase, providing accurate guidance to developers.
docs/learn/advanced/12-simulation.md
Outdated
| 1. Completely random where the initial state, module parameters and simulation parameters are **pseudo-randomly generated**. | ||
| 2. From a `genesis.json` file where the initial state and the module parameters are defined. This mode is helpful for running simulations on a known state such as a live network export where a new (mostly likely breaking) version of the application needs to be tested. | ||
| 3. From a `params.json` file where the initial state is pseudo-randomly generated but the module and simulation parameters can be provided manually. This allows for a more controlled and deterministic simulation setup while allowing the state space to still be pseudo-randomly simulated. The list of available parameters are listed [here](https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/x/simulation/client/cli/flags.go#L59-L78). | ||
|
|
||
| :::tip | ||
| These modes are not mutually exclusive. So you can for example run a randomly | ||
| generated genesis state (`1`) with manually generated simulation params (`3`). | ||
| These modes are not mutually exclusive. So you can for example run a randomly generated genesis state (`1`) with manually generated simulation params (`3`). |
There was a problem hiding this comment.
LGTM, but please correct the agreement error.
Reformatting the modes of simulation enhances clarity and emphasizes that they are not mutually exclusive.
However, there is an agreement error in the sentence "The list of available parameters are listed here." The subject "list" is singular, so the verb should be "is" instead of "are."
Please correct the sentence as follows:
-The list of available parameters are listed [here](https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/x/simulation/client/cli/flags.go#L59-L78).
+The list of available parameters is listed [here](https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/x/simulation/client/cli/flags.go#L59-L78).Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1. Completely random where the initial state, module parameters and simulation parameters are **pseudo-randomly generated**. | |
| 2. From a `genesis.json` file where the initial state and the module parameters are defined. This mode is helpful for running simulations on a known state such as a live network export where a new (mostly likely breaking) version of the application needs to be tested. | |
| 3. From a `params.json` file where the initial state is pseudo-randomly generated but the module and simulation parameters can be provided manually. This allows for a more controlled and deterministic simulation setup while allowing the state space to still be pseudo-randomly simulated. The list of available parameters are listed [here](https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/x/simulation/client/cli/flags.go#L59-L78). | |
| :::tip | |
| These modes are not mutually exclusive. So you can for example run a randomly | |
| generated genesis state (`1`) with manually generated simulation params (`3`). | |
| These modes are not mutually exclusive. So you can for example run a randomly generated genesis state (`1`) with manually generated simulation params (`3`). | |
| 1. Completely random where the initial state, module parameters and simulation parameters are **pseudo-randomly generated**. | |
| 2. From a `genesis.json` file where the initial state and the module parameters are defined. This mode is helpful for running simulations on a known state such as a live network export where a new (mostly likely breaking) version of the application needs to be tested. | |
| 3. From a `params.json` file where the initial state is pseudo-randomly generated but the module and simulation parameters can be provided manually. This allows for a more controlled and deterministic simulation setup while allowing the state space to still be pseudo-randomly simulated. The list of available parameters is listed [here](https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/x/simulation/client/cli/flags.go#L59-L78). | |
| :::tip | |
| These modes are not mutually exclusive. So you can for example run a randomly generated genesis state (`1`) with manually generated simulation params (`3`). |
Tools
LanguageTool
[grammar] ~41-~41: Possible agreement error.
Context: ...lated. The list of available parameters are listed [here](https://github.com/cosmos...(AGREEMENT_SENT_START)
github-actions
bot
added
C:x/distribution
julienrbrt
added
the
backport/v0.52.x
github-actions
bot
removed
C:x/distribution
Co-authored-by: Julián Toledano <JulianToledano@users.noreply.github.com>
Co-authored-by: Julián Toledano <JulianToledano@users.noreply.github.com> (cherry picked from commit 870cab5)
Description
this pr does a minor cleanup of docs around the simulator
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
FuzzFullAppSimulationcommand for enhanced testing using the Go fuzzer.Documentation
simsxpackage README.Bug Fixes